Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove references to "_aks" environment names #4693

Merged
merged 3 commits into from
Nov 21, 2024
Merged

Remove references to "_aks" environment names #4693

merged 3 commits into from
Nov 21, 2024

Conversation

Nitemaeric
Copy link
Contributor

Context

We've recently dropped "_aks" suffixes from AKS configurations.

We should remove the "_aks" suffixes from application code too.

Changes proposed in this pull request

  • Remove previous non-aks environment configurations.
  • Remove _aks suffix from environment names.

Guidance to review

Let's see if review works :D

@Nitemaeric Nitemaeric self-assigned this Nov 18, 2024
@Nitemaeric Nitemaeric requested a review from a team as a code owner November 18, 2024 13:56
@Nitemaeric Nitemaeric added deploy A Review App will be created for PRs with this label and removed deploy_v2 labels Nov 18, 2024
@Nitemaeric
Copy link
Contributor Author

Tech debt: Remove "_aks" suffixes from environment names

tomas-stefano
tomas-stefano approved these changes Nov 20, 2024
View reviewed changes
Copy link
Contributor

@tomas-stefano tomas-stefano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the app point of view, looks good 👍

Is there anything we need to do on infra side or did they already remove references to _aks as well?

Such as copy/moving the settings.yml for each environment and so on?

@RMcVelia
Copy link
Contributor

Could you also rename maintenance_page/manifests/env_aks to just env, or I can update on a separate pr if preferred.

@Nitemaeric Nitemaeric requested a review from RMcVelia November 21, 2024 10:27
@Nitemaeric
Copy link
Contributor Author

Could you also rename maintenance_page/manifests/env_aks to just env, or I can update on a separate pr if preferred.

Done!

I'm curious about how it knows how to pick up the changes though. The workflow file has only production and qa as options, yet the directories were named production_aks and qa_aks. Was this a bug?

@RMcVelia
Copy link
Contributor

Could you also rename maintenance_page/manifests/env_aks to just env, or I can update on a separate pr if preferred.

Done!

I'm curious about how it knows how to pick up the changes though. The workflow file has only production and qa as options, yet the directories were named production_aks and qa_aks. Was this a bug?

Thanks! The workflow would call env, but maintenance_page/scripts/failover.sh uses $CONFIG from global_config/env.sh which is now being changed from env_aks to env.

@Nitemaeric Nitemaeric merged commit eef9782 into main Nov 21, 2024
19 checks passed
@Nitemaeric Nitemaeric deleted the dd/remove-aks-from-rails-environments branch November 21, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomas-stefano tomas-stefano tomas-stefano approved these changes

@RMcVelia RMcVelia Awaiting requested review from RMcVelia

Assignees

@Nitemaeric Nitemaeric

Labels
deploy A Review App will be created for PRs with this label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Nitemaeric @RMcVelia @tomas-stefano